-
Notifications
You must be signed in to change notification settings - Fork 561
High availability & sentinel #101
Conversation
…iber which will handle connect/reconnection automatically to redis server.
…iber which will handle connect/reconnection automatically to redis server.
… changes to unix.
| //! ctor & dtor | ||
| #ifndef __CPP_REDIS_USE_CUSTOM_TCP_CLIENT | ||
| redis_connection(void); | ||
| redis_connection(std::uint32_t num_io_workers = 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to be ported to future_client too
includes/cpp_redis/redis_client.hpp
Outdated
| typedef std::function<void(reply&)> reply_callback_t; | ||
| redis_client& set_callback_runner(const std::function<void(reply&, const reply_callback_t& callback)>& callback); | ||
| redis_client& send(const std::vector<std::string>& redis_cmd, const reply_callback_t& callback = nullptr); | ||
| virtual redis_client& send(const std::vector<std::string>& redis_cmd, const reply_callback_t& callback = nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double check that
| namespace cpp_redis { | ||
|
|
||
| class redis_subscriber { | ||
| friend class ha_redis_subscriber; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double check that, kind of dislike use of friend. Is it necessary?
| void connect(const std::string& host = "127.0.0.1", std::size_t port = 6379, const disconnection_handler_t& disconnection_handler = nullptr); | ||
| void disconnect(bool wait_for_removal = false); | ||
| bool is_connected(void); | ||
| virtual void connect(const std::string& host = "127.0.0.1", std::size_t port = 6379, const disconnection_handler_t& disconnection_handler = nullptr, std::uint32_t timeout_msecs = 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check uses of virtual
install_deps.sh
Outdated
| @@ -0,0 +1,12 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be removed
|
Go ahead. I’m not sure what that is…
Mike Moening
*From:* Simon Ninon [mailto:notifications@github.com]
*Sent:* Friday, August 25, 2017 9:40 PM
*To:* Cylix/cpp_redis
*Cc:* Subscribed
*Subject:* Re: [Cylix/cpp_redis] High availability & sentinel (#101)
*@Cylix* commented on this pull request.
------------------------------
In install_deps.sh
<#101 (comment)>:
@@ -0,0 +1,12 @@
+#!/bin/sh
should be removed
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#101 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARc77Gr3HIsmeUgzez5T0-rigKw7Q_0gks5sb4V-gaJpZM4PCQHc>
.[image: Image removed by sender.]
|
sources/redis_client.cpp
Outdated
|
|
||
| redis_client& | ||
| redis_client::scan(int cursor, const std::string& pattern, int count, const reply_callback_t& reply_callback) { | ||
| if (pattern.size() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRY
|
Try taking it out and compiling. Then you will see why…
Mike Moening
*From:* Simon Ninon [mailto:notifications@github.com]
*Sent:* Friday, August 25, 2017 9:39 PM
*To:* Cylix/cpp_redis
*Cc:* Subscribed
*Subject:* Re: [Cylix/cpp_redis] High availability & sentinel (#101)
*@Cylix* commented on this pull request.
------------------------------
In includes/cpp_redis/redis_subscriber.hpp
<#101 (comment)>:
@@ -33,10 +33,12 @@
namespace cpp_redis {
class redis_subscriber {
+ friend class ha_redis_subscriber;
double check that, kind of dislike use of friend. Is it necessary?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#101 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARc77GiBkVTeohfTTA5ELKAXgEkoT2Iuks5sb4VdgaJpZM4PCQHc>
.[image: Image removed by sender.]
|
| #include <cpp_redis/redis_client.hpp> | ||
| #include <cpp_redis/redis_error.hpp> | ||
| #include <cpp_redis/redis_subscriber.hpp> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not be changed
|
|
||
| #include <cpp_redis/redis_client.hpp> | ||
| #include <cpp_redis/redis_error.hpp> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not be changed
|
The virtual is necessary since we are overriding the base classes connect.
We also want to allow this class to potentially be overridden as well by
another down the road.
Mike Moening
*From:* Simon Ninon [mailto:notifications@github.com]
*Sent:* Friday, August 25, 2017 9:40 PM
*To:* Cylix/cpp_redis
*Cc:* Subscribed
*Subject:* Re: [Cylix/cpp_redis] High availability & sentinel (#101)
*@Cylix* commented on this pull request.
------------------------------
In includes/cpp_redis/redis_subscriber.hpp
<#101 (comment)>:
@@ -48,15 +50,15 @@ class redis_subscriber {
public:
//! handle connection
typedef std::function<void(redis_subscriber&)> disconnection_handler_t;
- void connect(const std::string& host = "127.0.0.1", std::size_t
port = 6379, const disconnection_handler_t& disconnection_handler =
nullptr);
- void disconnect(bool wait_for_removal = false);
- bool is_connected(void);
+ virtual void connect(const std::string& host = "127.0.0.1",
std::size_t port = 6379, const disconnection_handler_t&
disconnection_handler = nullptr, std::uint32_t timeout_msecs = 0);
check uses of virtual
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#101 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARc77IHK2SNjcfZnWd678uSoNADW9EMZks5sb4V6gaJpZM4PCQHc>
.[image: Image removed by sender.]
|
…ke interface cleaner. rename redis_ prefix to classes
…e with callbacks execution)
|
This PR is now successfully reworked! Considering the initial changes, here are the main changes:
Some other big changes not related to the original changes have been also brought and are listed in the CHANGELOG. This will be released under 4.0.0. Please let me know if you have any feedbacks :) Thanks again for your great work, it has been really inspiring and I believe it is a really meaningful and useful contribution to cpp_redis! So thanks a lot Mike, I really appreciate your constant involvement in this library! Best :) |
* High availability, sentinel, design improvement (#101) * Added sentiel and high availability(HA) versions of client and subscriber which will handle connect/reconnection automatically to redis server. * Merged changes * clang format * bump tacopie * unix compilation. will need some time to code review and port windows changes to unix. * clean unecessary changes. merge future_client into redis_client to make interface cleaner. rename redis_ prefix to classes * merge ha_client into client, refactore architecture. * merge ha_subscriber into subscriber * try_commit instead of commit in disconnection flow * doxygen * tacopie link & documentation & compilation fix * fix reconnection process * fix unit tests * clean code sentinel & make it consistent (avoid regression for example with callbacks execution) * ability to change number of io service workers at runtime * ha redis client example * doxygen doc * switch ZADD score param from map to multimap #107 * update CHANGELOG.md for 4.0.0 release * bump tacopie * [4.0.0] update README
Cpp Redis changes in order to implement the Redis high availability and sentinel features.
This will need some time to code review everything, port windows changes to unix and test it.
Full credits to Mike Moeining for his amazing work!